Skip to content

Conversation

@ftalpo
Copy link
Collaborator

@ftalpo ftalpo commented Jul 28, 2025

@dobbi84 prova a vedere come ti sembra ora.

Ho semplificato il codice sotto utils.command.go per renderlo più simile a prima.

Ho implementato una mossa smart che mi era sfuggita: invece che avere un unico NvmeCollector in pkg.collector.go, ho diviso i collector in 3 (InfoMetricCollector, un LogMetricCollector per le metriche non-OCP e un LogMetricCollector per le metriche OCP), e ora newNvmeCollector in cmd/collector.go istanzia un CompositeCollector, che durante Describe e Collect non fa altro che iterare su tutti i MetricCollector che si tiene nella sua lista.

Così facendo, oltre a rimuovere del codice duplicato come mi avevi fatto notare, quando OCP è enabled aggiungiamo il LogMetricCollector anche per le metriche OCP, e quando è disabled lo saltiamo direttamente (questo succede in cmd/collector.go), senza bisogno di fare if checks strani dentro Collect e Describe.

ftalpo added 7 commits July 18, 2025 16:54
- implement a OOP structure with MetricProvider objects that return prometheus Metrics from the json log result

- implement an agnostic NvmeCollector that stores MetricProviders instances and delegates handling the details of Collect and Describe to them

- move NvmeCollector instantiation in a convenience function in the main package
…CP metrics inclusion logic: now, if ocp is disabled, we simply don't include the OCP LogMetricCollector in the list. Simplify utils.command.go logic
@ftalpo ftalpo requested a review from LemonySnippet as a code owner July 28, 2025 09:38
@ftalpo ftalpo marked this pull request as draft July 28, 2025 16:03
@ftalpo ftalpo force-pushed the ftalpo/oop_refactor branch from 405410f to e733836 Compare July 28, 2025 16:04
@dobbi84 dobbi84 marked this pull request as ready for review July 29, 2025 08:55
@dobbi84 dobbi84 self-assigned this Jul 29, 2025
@dobbi84 dobbi84 added the enhancement New feature or request label Jul 29, 2025
@dobbi84 dobbi84 merged commit d90ad60 into main Jul 29, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants